Revert "feat: geofence-based geographic targeting for programs"#271
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the GIS and hazard reporting modules by removing the OGC API - Processes endpoints and replacing them with direct spatial query endpoints, reverting the CAP vocabulary severity migration back to a legacy selection scale, and reverting demographic disaggregation dimensions to boolean flags. Additionally, the Luzon geodata demo module has been removed in favor of a curated set of Philippine areas. The review feedback highlights a critical safety improvement in the map widget's draw event handler, recommending the use of optional chaining on the event object rather than an unguarded call to retrieve all features.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| function updateArea(e) { | ||
| console.log(e); | ||
| var data = self.draw.getAll(); | ||
| self.props.record.update({ | ||
| [self.props.name]: JSON.stringify(data.features[0].geometry), | ||
| }); | ||
| } |
There was a problem hiding this comment.
When handling map draw events, avoid using unguarded getAll().features access. Instead, safely read the affected features directly from the event object using optional chaining (e.g., e?.features?.[0]) and guard on geometry before performing updates to prevent runtime errors.
| function updateArea(e) { | |
| console.log(e); | |
| var data = self.draw.getAll(); | |
| self.props.record.update({ | |
| [self.props.name]: JSON.stringify(data.features[0].geometry), | |
| }); | |
| } | |
| function updateArea(e) { | |
| const geometry = e?.features?.[0]?.geometry; | |
| if (geometry) { | |
| self.props.record.update({ | |
| [self.props.name]: JSON.stringify(geometry), | |
| }); | |
| } | |
| } |
References
- When handling map draw events (such as
draw.createordraw.update), avoid using unguardedgetAll().featuresaccess. Instead, safely read the affected features directly from the event object using optional chaining (e.g.,e?.features?.[0]) and guard ongeometrybefore performing updates to prevent runtime errors.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| registrants_by_area = {} | ||
| for registrant in registrants: | ||
| area_id = registrant.area_id.id | ||
| if area_id not in registrants_by_area: |
| # Count total registrants with area_id | ||
| total_count = Partner.search_count([("is_registrant", "=", True), ("area_id", "!=", False)]) | ||
| # Get all registrants with an area_id | ||
| registrants = Partner.search( |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 19.0 #271 +/- ##
==========================================
- Coverage 75.31% 75.09% -0.22%
==========================================
Files 1098 1093 -5
Lines 65317 63448 -1869
==========================================
- Hits 49191 47649 -1542
+ Misses 16126 15799 -327
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d38ff9d7a8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # Params ordered to match SQL: filter params, geometry, population filter | ||
| params = filter_params + [geometry_json] + pop_params | ||
| # Add geometry parameter at the beginning | ||
| params = [geometry_json] + params[1:] |
There was a problem hiding this comment.
When filters.is_group is provided, the generated SQL has the p.is_group = %s placeholder before the ST_GeomFromGeoJSON(%s) placeholder, but this line still sends the GeoJSON value first. That binds a JSON string to the boolean comparison, causing the coordinate query to error and fall back to the area path; in deployments relying on point coordinates or with incomplete area polygons, filtered /gis/query/statistics results become empty or inaccurate instead of using the matching coordinates.
Useful? React with 👍 / 👎.
| HTTPException: 403 if missing scope, 422 if validation fails | ||
| """ | ||
| # Check geofence scope | ||
| if not api_client.has_scope("gis", "geofence"): |
There was a problem hiding this comment.
This write endpoint still requires api_client.has_scope("gis", "geofence"), but the same change removes the geofence action from spp.api.client.scope's selection. After an upgrade, geofence-only scopes are no longer valid/can be cascaded away, and admins can only satisfy this check by granting broad gis:all; clients intended to create/delete geofences without full GIS access will receive 403s.
Useful? React with 👍 / 👎.
| # Verify the module is actually installed now | ||
| state = _get_module_state(profile, module_name) | ||
| if state == "installed": |
There was a problem hiding this comment.
For custom demo values such as ./spp start --wipe --demo=a,b, ODOO_INIT_MODULES is correctly set to the comma-separated list, but this check now searches ir.module.module for the literal name a,b. Odoo stores module names separately, so _get_module_state returns NOT_FOUND and the CLI waits until timeout even after the modules finish installing; use one module from the list, as the previous code did, when verifying completion.
Useful? React with 👍 / 👎.
Mainline depends on this piece of #76: PR #160 (fix/name-format-on-create-write) overwrites explicit names on create/write, and spp_demo's demo-stories tests assert explicit names survive. The guard added in #76 (skip formatting when vals contains an explicit name) is what keeps spp_demo green on 19.0. Reverting it fails CI (3 failures in spp_demo test_demo_stories), so this bugfix stays; everything else from #76 remains reverted and will be re-landed in scoped PRs.
|
CI was red on this revert: Root cause: mainline already depends on one piece of #76 — the Pushed |
Bring the branch up to date with 19.0 (30 commits, incl. the geofence partial-revert #271). Conflict in spp_hazard/views/hazard_incident_views.xml resolved by adopting 19.0's Impacts-page rework while preserving this PR's security `groups=` gate on the Impacts page and the 'Affected' stat button. Auto-merged manifest (kept our 2.0.3 security bump) and tests/__init__.py (kept our test_acl_group_user import, took 19.0's removal of test_alert_ingestion).
3 participants
Reverts #76